-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[libc++] Applied [[nodiscard]] to some general utilities
#169322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[libc++] Applied [[nodiscard]] to some general utilities
#169322
Conversation
`[[nodiscard]]` should be applied to functions where discarding the return value is most likely a correctness issue. - https://libcxx.llvm.org/CodingGuidelines.html#apply-nodiscard-where-relevant The following has been annotated in this patch: - [x] `bind_back`, `bind_front`, `bind` - [x] `function`, `mem_fn` - [x] `reference_wrapper`
|
@llvm/pr-subscribers-libcxx Author: Hristo Hristov (H-G-Hristov) Changes
The following has been annotated in this patch:
Full diff: https://github.com/llvm/llvm-project/pull/169322.diff 7 Files Affected:
diff --git a/libcxx/include/__functional/bind.h b/libcxx/include/__functional/bind.h
index def9e4c4ec7a9..328dc3bf3dabc 100644
--- a/libcxx/include/__functional/bind.h
+++ b/libcxx/include/__functional/bind.h
@@ -278,14 +278,14 @@ template <class _Rp, class _Fp, class... _BoundArgs>
struct is_bind_expression<__bind_r<_Rp, _Fp, _BoundArgs...> > : public true_type {};
template <class _Fp, class... _BoundArgs>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __bind<_Fp, _BoundArgs...>
+[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __bind<_Fp, _BoundArgs...>
bind(_Fp&& __f, _BoundArgs&&... __bound_args) {
typedef __bind<_Fp, _BoundArgs...> type;
return type(std::forward<_Fp>(__f), std::forward<_BoundArgs>(__bound_args)...);
}
template <class _Rp, class _Fp, class... _BoundArgs>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __bind_r<_Rp, _Fp, _BoundArgs...>
+[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __bind_r<_Rp, _Fp, _BoundArgs...>
bind(_Fp&& __f, _BoundArgs&&... __bound_args) {
typedef __bind_r<_Rp, _Fp, _BoundArgs...> type;
return type(std::forward<_Fp>(__f), std::forward<_BoundArgs>(__bound_args)...);
diff --git a/libcxx/include/__functional/bind_back.h b/libcxx/include/__functional/bind_back.h
index e44768d2283c0..41177144d81fe 100644
--- a/libcxx/include/__functional/bind_back.h
+++ b/libcxx/include/__functional/bind_back.h
@@ -64,7 +64,7 @@ _LIBCPP_HIDE_FROM_ABI constexpr auto __bind_back(_Fn&& __f, _Args&&... __args) n
# if _LIBCPP_STD_VER >= 23
template <class _Fn, class... _Args>
-_LIBCPP_HIDE_FROM_ABI constexpr auto bind_back(_Fn&& __f, _Args&&... __args) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto bind_back(_Fn&& __f, _Args&&... __args) {
static_assert(is_constructible_v<decay_t<_Fn>, _Fn>, "bind_back requires decay_t<F> to be constructible from F");
static_assert(is_move_constructible_v<decay_t<_Fn>>, "bind_back requires decay_t<F> to be move constructible");
static_assert((is_constructible_v<decay_t<_Args>, _Args> && ...),
diff --git a/libcxx/include/__functional/bind_front.h b/libcxx/include/__functional/bind_front.h
index 87ef3affe80b6..427accf96339d 100644
--- a/libcxx/include/__functional/bind_front.h
+++ b/libcxx/include/__functional/bind_front.h
@@ -43,7 +43,7 @@ struct __bind_front_t : __perfect_forward<__bind_front_op, _Fn, _BoundArgs...> {
template <class _Fn, class... _Args>
requires is_constructible_v<decay_t<_Fn>, _Fn> && is_move_constructible_v<decay_t<_Fn>> &&
(is_constructible_v<decay_t<_Args>, _Args> && ...) && (is_move_constructible_v<decay_t<_Args>> && ...)
-_LIBCPP_HIDE_FROM_ABI constexpr auto bind_front(_Fn&& __f, _Args&&... __args) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto bind_front(_Fn&& __f, _Args&&... __args) {
return __bind_front_t<decay_t<_Fn>, decay_t<_Args>...>(std::forward<_Fn>(__f), std::forward<_Args>(__args)...);
}
diff --git a/libcxx/include/__functional/function.h b/libcxx/include/__functional/function.h
index c768fd90d01b4..121417f90ff01 100644
--- a/libcxx/include/__functional/function.h
+++ b/libcxx/include/__functional/function.h
@@ -672,11 +672,11 @@ class function<_Rp(_ArgTypes...)>
# if _LIBCPP_HAS_RTTI
// function target access:
- _LIBCPP_HIDE_FROM_ABI const std::type_info& target_type() const _NOEXCEPT;
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const std::type_info& target_type() const _NOEXCEPT;
template <typename _Tp>
- _LIBCPP_HIDE_FROM_ABI _Tp* target() _NOEXCEPT;
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _Tp* target() _NOEXCEPT;
template <typename _Tp>
- _LIBCPP_HIDE_FROM_ABI const _Tp* target() const _NOEXCEPT;
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const _Tp* target() const _NOEXCEPT;
# endif // _LIBCPP_HAS_RTTI
};
diff --git a/libcxx/include/__functional/mem_fn.h b/libcxx/include/__functional/mem_fn.h
index 690393988c5a5..1c9340c4f4183 100644
--- a/libcxx/include/__functional/mem_fn.h
+++ b/libcxx/include/__functional/mem_fn.h
@@ -43,7 +43,8 @@ class __mem_fn : public __weak_result_type<_Tp> {
};
template <class _Rp, class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __mem_fn<_Rp _Tp::*> mem_fn(_Rp _Tp::*__pm) _NOEXCEPT {
+[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __mem_fn<_Rp _Tp::*>
+mem_fn(_Rp _Tp::* __pm) _NOEXCEPT {
return __mem_fn<_Rp _Tp::*>(__pm);
}
diff --git a/libcxx/include/__functional/reference_wrapper.h b/libcxx/include/__functional/reference_wrapper.h
index 148703b21d84a..1bb33cb033dcb 100644
--- a/libcxx/include/__functional/reference_wrapper.h
+++ b/libcxx/include/__functional/reference_wrapper.h
@@ -58,11 +58,11 @@ class reference_wrapper : public __weak_result_type<_Tp> {
// access
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 operator type&() const _NOEXCEPT { return *__f_; }
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 type& get() const _NOEXCEPT { return *__f_; }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 type& get() const _NOEXCEPT { return *__f_; }
// invoke
template <class... _ArgTypes>
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __invoke_result_t<type&, _ArgTypes...>
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __invoke_result_t<type&, _ArgTypes...>
operator()(_ArgTypes&&... __args) const
#if _LIBCPP_STD_VER >= 17
// Since is_nothrow_invocable requires C++17 LWG3764 is not backported
@@ -128,23 +128,25 @@ reference_wrapper(_Tp&) -> reference_wrapper<_Tp>;
#endif
template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 reference_wrapper<_Tp> ref(_Tp& __t) _NOEXCEPT {
+[[nodiscard]] inline _LIBCPP_HIDE_FROM_ABI
+_LIBCPP_CONSTEXPR_SINCE_CXX20 reference_wrapper<_Tp> ref(_Tp& __t) _NOEXCEPT {
return reference_wrapper<_Tp>(__t);
}
template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 reference_wrapper<_Tp>
+[[nodiscard]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 reference_wrapper<_Tp>
ref(reference_wrapper<_Tp> __t) _NOEXCEPT {
return __t;
}
template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 reference_wrapper<const _Tp> cref(const _Tp& __t) _NOEXCEPT {
+[[nodiscard]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 reference_wrapper<const _Tp>
+cref(const _Tp& __t) _NOEXCEPT {
return reference_wrapper<const _Tp>(__t);
}
template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 reference_wrapper<const _Tp>
+[[nodiscard]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 reference_wrapper<const _Tp>
cref(reference_wrapper<_Tp> __t) _NOEXCEPT {
return __t;
}
diff --git a/libcxx/test/libcxx/diagnostics/functional.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/functional.nodiscard.verify.cpp
index 4307976e9e442..d07ea7d2e0f5f 100644
--- a/libcxx/test/libcxx/diagnostics/functional.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/functional.nodiscard.verify.cpp
@@ -6,7 +6,7 @@
//
//===----------------------------------------------------------------------===//
-// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: c++03
// check that <functional> functions are marked [[nodiscard]]
@@ -16,5 +16,47 @@
void test() {
int i = 0;
- std::identity()(i); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+ // Function wrappers
+
+#if !defined(TEST_HAS_NO_RTTI)
+ std::function<void(int)> f;
+ const std::function<void(int)> cf;
+
+ f.target_type(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ f.target<void(int)>(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ cf.target<void(int)>(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
+ struct ZMT {
+ void member_function() {};
+ };
+ // expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+ std::mem_fn(&ZMT::member_function);
+
+ // Identity
+
+ std::identity{}(i); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+ // Partial function application
+
+#if TEST_STD_VER >= 23
+ // expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+ std::bind_back([](int a) { return a; }, 94);
+ // expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+ std::bind_front([](int a) { return a; }, 94);
+#endif
+ // expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+ std::bind([](int a) { return a; }, 94);
+ // expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+ std::bind<float>([](int a) { return a; }, 94);
+
+ // Reference wrappers
+ std::reference_wrapper<int> rw{i};
+ rw.get(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ std::function<int(int)> vf;
+ auto rwf = std::ref(vf);
+ rwf(1); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+ std::ref(i); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ std::cref(i); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
}
|
|
Is it intended to add |
Yes. I am implementing the changes in chunks trying to balance between what's more often used while keeping the PRs neither too small nor too big. |
[[nodiscard]]should be applied to functions where discarding the return value is most likely a correctness issue.The following functions/classes have been annotated in this patch:
bind_back,bind_front,bindfunction,mem_fnreference_wrapper